Don't increase scrollable size for zero width tables#3283
Conversation
| if (width > 1) | ||
| layoutTableTree(table, width, area, tableWidth < area.width); | ||
| layoutTableTree(table, width, area, tableWidth > 0 && tableWidth < area.width); |
There was a problem hiding this comment.
| if (width > 1) | |
| layoutTableTree(table, width, area, tableWidth < area.width); | |
| layoutTableTree(table, width, area, tableWidth > 0 && tableWidth < area.width); | |
| if (width > 1 && tableWidth > 0) | |
| layoutTableTree(table, width, area, tableWidth < area.width); |
This seems more logical to me, we already check if the areas width is larger than 1 (why not zero? anyways ...) so it seems to already want to skip under this condition, having the table has no size at all seems a valid extension of such checks.
There was a problem hiding this comment.
Assuming we merge this one and it will introduce regressions, would you then agree to merge eclipse-equinox/p2#947 instead?
I honestly don't have more time to spend on debugging the "Trust" preference page.
There was a problem hiding this comment.
I can relate to not having time, so I empathize. I wold much prefer to see this cycle broken in this part of the code.
You made a suggestion but I don't really understand it.
Is the indentation off on the new line 256?
There was a problem hiding this comment.
This seems more logical to me,
I haven't noticed that you have proposed different change. No, your proposal will not work, layoutTableTree() call is really needed. My proposal avoids some handling inside that method.
There was a problem hiding this comment.
I reviewed it in my workspace and it looks fine to me. Please proceed. (And thank you!).
| if (width > 1) | ||
| layoutTableTree(table, width, area, tableWidth < area.width); | ||
| layoutTableTree(table, width, area, tableWidth > 0 && tableWidth < area.width); |
There was a problem hiding this comment.
I reviewed it in my workspace and it looks fine to me. Please proceed. (And thank you!).
This seem to fix StackOverflow on "Trust" table on Linux, see eclipse-equinox/p2#946. No idea how and if this affect everything else.
iloveeclipse
force-pushed
the
stack_overflow_on_table_layout
branch
from
fbe80a2 to
c99cbab
Compare
3 participants
This seem to fix StackOverflow on "Trust" table on Linux, see eclipse-equinox/p2#946.
No idea how and if this affect everything else.